-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deploy: container history #578
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a great start! 😍 left some food for thought - ill have a look again tomorrow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Make sure to test it.
args args | ||
wantErr bool | ||
}{ | ||
{"valid project build", args{"projectB", DeploymentMetadata{"hash", "ID", "status", "time"}, 2}, false}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might want to add some potential error cases here - what if the project name is empty? etc.
Codecov Report
@@ Coverage Diff @@
## master #578 +/- ##
=========================================
- Coverage 56.33% 55.82% -0.5%
=========================================
Files 68 68
Lines 3299 3372 +73
=========================================
+ Hits 1858 1882 +24
- Misses 1205 1248 +43
- Partials 236 242 +6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seifghazi so close! looks like there's just one last thing - a lint error:
https://travis-ci.org/ubclaunchpad/inertia/jobs/513613829#L527
+/home/travis/gopath/src/github.com/ubclaunchpad/inertia/daemon/inertiad/project/data.go:150:1: comment on exported method DeploymentDataManager.AddProjectBuildData should be of the form "AddProjectBuildData ..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's all green! awesome work, and congrats on your first PR 🎉
🎟️ Ticket(s): for #293
👷 Changes
Added containerBucket to manage container history for a deployed projects.
Once a project is successfully deployed, the bucket is updated with the container ID and the git project's most recent commit hash. The commit hash can be later be used to identify successful
builds and rollback to prior version when needed.
Before moving forward we might need to refactor data.go and extract env and container functionality to separate modules.
🔦 Testing Instructions
Explain how to test your changes, if applicable.